-
Notifications
You must be signed in to change notification settings - Fork 0
add eta quat struct and utilities for quaternion pid #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #17 +/- ##
==========================================
+ Coverage 88.38% 98.10% +9.71%
==========================================
Files 6 3 -3
Lines 594 369 -225
Branches 213 70 -143
==========================================
- Hits 525 362 -163
Misses 4 4
+ Partials 65 3 -62
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Q3rkses
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other than preferences i think it LGTM
| 0.90238159, -0.90929743, -0.35017549, -0.2248451; | ||
| Eigen::Matrix3d result = get_rotation_matrix(roll, pitch, yaw); | ||
| EXPECT_NEAR(0.0, matrix_norm_diff(expected, result), 0.01); | ||
| EXPECT_TRUE(result.isApprox(expected, 1e-8)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1e-8, 1e-12, 1e-3 seem a bit arbitrary. One maybe we should unify them or two maybe we should simply declare them as constants at the top.
#define MAGIC_TOLERANCE_1 1e-12,
with suitable name like strict and loose bound or more specific like controller or transformation tolerance ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah but its just test code so who cares 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thats why i approved ¯_(ツ)_/¯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my leader
|
🎉 This PR is included in version 1.6.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Some new additions you can use in your quat pid @ppakr @Q3rkses